Skip to content

Reduce scan startup time in large repositories - #301

Merged
lelia merged 16 commits into
mainfrom
lelia/reduce-scan-startup-time
Aug 19, 2026
Merged

lelia merged 16 commits into
mainfrom
lelia/reduce-scan-startup-time

Conversation

@lelia

@lelia lelia commented Aug 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Reduce Python CLI scan setup time in large repositories by replacing repeated filesystem traversal and broad Git fetching with single-pass, local-first implementations. Add native Buildkite pull-request context and enough diff-scan telemetry to distinguish client work from backend comparison time.

In an affected Buildkite pipeline, the preview reduced the two local bottlenecks from roughly 8 minutes combined to about 3 seconds:

Phase Before Preview
Git initialization ~4m 10s 1.65s
Manifest discovery ~4m 06s 1.30s

The preview found the same 765 manifests after visiting 175,045 files. The remaining end-to-end time was dominated by backend scan comparison and is being investigated separately.

Changes

  • Discover supported manifests with one streaming os.walk() per scan root, pruning excluded directories and .git before descent. Cache successfully loaded patterns and reuse discovery results for --sub-path scans.
  • Remove unconditional git fetch --all. Prefer refs already in the checkout and fetch only a missing base or head ref when a comparison requires it.
  • Recognize Buildkite commit, branch, and pull-request variables directly, including GitHub SCM comment context, without GitHub Actions variable shims.
  • Add INFO-level timings and counters for CLI initialization, Git setup, changed-file detection, pattern loading, manifest discovery, and diff polling.
  • Log the diff-scan ID and report URL, cap polling intervals at 10 seconds, and document the API token scopes required by the diff-scan flow.
  • Omit unchanged comparison artifacts when no enabled output consumes them, avoiding a large response and object graph on ordinary PR runs. Preserve the full result for strict blocking, GitLab security output, license generation, and FOSSA output.
  • Make opt-in Docker previews build from the downloaded wheel and publish both amd64 and arm64 images.

Behavior Notes

  • Scan routing is unchanged: the CLI still creates a full report when no supported manifest appears in the changed-file set.
  • .git is intentionally excluded from manifest discovery.
  • Directory exclusions now honor globs, so existing defaults such as *.egg-info are applied as intended.
  • The unchanged-artifact optimization reduces response size and client memory; it does not skip the underlying backend comparison.

Validation

  • Validated a Docker preview in the affected Buildkite pipeline: Git setup and manifest discovery fell from ~8m 16s to ~3s combined with equivalent manifest results.
  • Matching-parity coverage spans every built-in manifest pattern plus casing, nested patterns, hidden directories, exclusions, symlinks, multiple roots, sorting, and deduplication.
  • Git coverage includes local-ref preference, targeted-fetch fallback, Buildkite, GitHub Actions, GitLab CI, Bitbucket Pipelines, detached HEAD, and non-PR execution.
  • Memory regression coverage verifies discovery remains streaming and bounded by directory width and the result set rather than repository size.
  • Live API validation confirms omit_unchanged removes unused artifacts while preserving them for every output mode that consumes them.
  • 476 passed, 2 skipped; Ruff clean on all changed Python files; source and wheel builds verified.

Ref: CE-379


Note

Medium Risk
Changes sit on the critical path for manifest discovery, Git PR baselines, and diff-scan payloads; incorrect parity or gating could mis-route scans or strip data from strict-blocking/license outputs, though broad unit tests and streaming fallbacks limit blast radius.

Overview
Release 2.6.6 cuts local scan startup on large repos by replacing repeated per-pattern rglob manifest discovery with one pruned os.walk per scan root (cached API patterns, basename prefilter, directory globs like *.egg-info), and by reusing manifest lists from --sub-path pre-checks during scan creation.

Git no longer runs git fetch --all at init. PR/MR changed-file detection uses local refs first and targeted fetches only when a base or history is missing. Buildkite commit, branch, and PR variables are read natively (including default-branch vs PR routing), and --scm github on Buildkite can derive GitHub comment context from BUILDKITE_* when GitHub Actions vars are absent.

Diff comparisons always load artifacts through the cached diff-scan GET (eager create/list payloads cannot skip it). When no enabled output needs unchanged packages, polls request omit_unchanged=true; polling max interval is 10s (down from 30s), with INFO logs for diff-scan id, report URL, poll counts, and stage timings across init, Git, discovery, and registration.

CI/docs: PR Docker previews build from the workspace context with linux/amd64 and linux/arm64 (QEMU). Troubleshooting and Buildkite docs cover diff-scan token scopes and checkout expectations.

Reviewed by Cursor Bugbot for commit b812a18. Configure here.

lelia added 6 commits August 12, 2026 17:35
find_files() started a separate recursive rglob traversal for every expanded
manifest pattern, so a scan re-walked each root once per pattern and only
filtered excluded directories after descending into them. Replace that with one
os.walk() per scan root:

- Expand and case-fold all active patterns once, then match in memory.
- Prune excluded directories, including .git, before descending.
- Reject non-manifests on the basename alone (one set lookup plus one compiled
  glob alternation) before building a relative path or running a path match.
- Cache supported manifest patterns per Core instance, but only when the API
  lookup succeeds, so a transient failure does not pin the run to the smaller
  local fallback pattern set.
- Emit INFO durations for organization setup, pattern retrieval and discovery,
  with files/directories visited, directories pruned and manifests found.

Matching behaviour is unchanged apart from intentionally excluding .git
metadata. Adds parity tests against the previous rglob implementation for every
built-in ecosystem and pattern, covering case-insensitivity, brace expansion,
nested patterns, dot-directories, exclusions, inclusions, symlinks, excluded
ecosystems, multiple roots, sorting and deduplication, plus an opt-in benchmark
that asserts old/new result equality on a synthetic large-monorepo fixture.

Ref: CE-379
Git.__init__() ran `git fetch --all` on every invocation, pulling every remote
branch and tag before changed-file detection even began. Resolve commit and
branch metadata locally instead, and for pull-request comparisons prefer refs
already present in the checkout, fetching a single base or head ref only when it
is missing.

Also recognises Buildkite's native BUILDKITE_COMMIT, BUILDKITE_BRANCH,
BUILDKITE_PULL_REQUEST and BUILDKITE_PULL_REQUEST_BASE_BRANCH so Buildkite jobs
can calculate a complete base-to-head changed-file range without mapping their
environment onto GitHub Actions variable names. Buildkite is checked before
GitHub because some pipelines deliberately export GitHub-compatible variables.

Adds INFO durations for Git initialisation, changed-file detection and each
fetch, including the ref requested and why. Existing GitHub Actions, GitLab CI,
Bitbucket Pipelines and local behaviour is preserved; tests cover local-ref
preference, absence of an unconditional fetch, the targeted-fetch fallback, all
four CI providers, and non-PR and detached-HEAD execution.

Ref: CE-379
`--scm github` read its configuration solely from GITHUB_* variables, so
Buildkite users had to shim every one of them to get PR comments. Fall back to
Buildkite's own variables when the GITHUB_* equivalents are absent: PR number,
commit, branch, checkout path, commit message, build creator, and owner/repository
parsed from BUILDKITE_REPO (preferring the pipeline repository over a
contributor's fork). Explicit GITHUB_* and PR_NUMBER values still take priority,
and GitHub Enterprise remains configurable via GITHUB_API_URL.

A running Buildkite PR build maps to the supported `synchronize` comment path,
and a non-PR build maps to `push`, so event routing is unchanged. Default-branch
detection requires an actual branch name rather than treating two unset variables
as a match, which would otherwise mark any build as the default branch and
overwrite the repository baseline.

Ref: CE-379
The --sub-path routing pre-check walked every selected path to decide whether any
manifests existed, then discarded the result so scan creation walked the same
paths again. Retain and reuse it.

Apply --excluded-ecosystems before the pre-check rather than after, so every
find_files() call in a run sees the same ecosystem filter.

Add an INFO duration for CLI run registration, and replace the
"No Manifest files changed" line with wording that describes the decision being
made: no supported manifest was detected in the changed-file set, so a full
report is created. Scan-routing semantics are unchanged.

Ref: CE-379
Bumped via .hooks/sync_version.py so __init__.py, pyproject.toml and uv.lock
stay in sync, and moved the changelog entry under a 2.6.5 heading.

Ref: CE-379
@lelia
lelia temporarily deployed to socket-firewall August 13, 2026 00:43 — with GitHub Actions Inactive
@lelia lelia added publish-preview Publish a CLI preview to TestPyPI. publish-docker-preview Publish `socketdev/cli:pr-<number>` to DockerHub labels Aug 13, 2026
@github-actions

Copy link
Copy Markdown

🚀 CLI preview published: socketsecurity==2.6.5.dev3165619303101

pip install --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple socketsecurity==2.6.5.dev3165619303101

TestPyPI's package index can take several minutes to expose a newly uploaded version.

The publish-docker job downloads the built wheel to ./dist, but the build step
omitted `context`, so docker/build-push-action used its default Git context.
Buildx then cloned the repository as the build context, where ./dist does not
exist, and `COPY dist/socketsecurity-*.whl` failed with
"lstat /dist: no such file or directory".

Set `context: .` so the build uses the workspace the artifact was downloaded
into. This also makes the job's existing trust boundary hold as documented: the
context is now the default-branch checkout rather than the pull-request ref, so
Dockerfile.preview is read from trusted code and the pull request still enters
the image only through the built wheel.

Pre-existing; the TestPyPI half of the workflow is unaffected.
@lelia lelia added publish-docker-preview Publish `socketdev/cli:pr-<number>` to DockerHub and removed publish-docker-preview Publish `socketdev/cli:pr-<number>` to DockerHub labels Aug 13, 2026
@lelia
lelia temporarily deployed to socket-firewall August 13, 2026 01:03 — with GitHub Actions Inactive
@github-actions

Copy link
Copy Markdown

🐳 Docker preview published: socketdev/cli:pr-301

This mutable tag is only created when a Docker preview is explicitly requested.

The preview image was amd64-only while the release and stable images are built
for linux/amd64,linux/arm64, so a preview tag could not stand in for
socketdev/cli:latest on arm64 hosts without emulation.

Match the release arch matrix and enable QEMU so the arm64 layer can be built on
an amd64 runner. Previews are opt-in via label, so the extra build time is an
acceptable tradeoff for making the tag a drop-in replacement.
@lelia
lelia temporarily deployed to socket-firewall August 15, 2026 01:29 — with GitHub Actions Inactive
@lelia lelia added publish-docker-preview Publish `socketdev/cli:pr-<number>` to DockerHub and removed publish-docker-preview Publish `socketdev/cli:pr-<number>` to DockerHub labels Aug 15, 2026
…utable

A finished comparison could sit unobserved for up to 30s between polls, which is
dead time on every PR job. Lower the ceiling to 10s: a multi-minute comparison
costs roughly 2x the polls while cutting worst-case dead time to 10s.

Diff scans now log their ID, poll count, and the wait before the final poll at
INFO. Previously the ID was debug-only, so a slow comparison in a customer CI log
could not be tied back to a server-side diff scan, and there was no way to tell
backend comparison time apart from time the result spent ready-but-unpolled.

Also document the diff-scans token scopes. A token missing them still completes
the scan, silently falling back to the streaming comparison, which differs in both
transport and payload (cached diff-scan responses always embed per-package license
details; the streaming path requests a lean payload).

Ref: CE-379
@lelia
lelia temporarily deployed to socket-firewall August 15, 2026 01:53 — with GitHub Actions Inactive
PR/MR runs logged the head and new scan IDs but no link to the result, so a CI log
gave no way to reach the report. Log the diff report URL where it is computed, so
every diff flow gets it rather than only the full-scan-only branches.

Also add a regression test asserting manifest discovery's peak allocation stays
bounded by the widest single directory and the result set rather than by repository
size. Measured against the per-pattern rglob approach this replaced, on a tree of
59,300 files including one 50,000-entry directory: 3.25 MB peak vs 10.72 MB.
os.walk keeps a list of names per directory where rglob materialised DirEntry
objects and a Path per candidate, so the single-pass walk allocates strictly less.

Ref: CE-379
@lelia
lelia deployed to socket-firewall August 15, 2026 01:59 — with GitHub Actions Active
Probed the live API against an existing diff scan to confirm what the polling path
can and cannot ask for:

- omit_license_details is ignored when cached=true, as the existing comment said.
  License fields remain in the response.
- omit_unchanged IS honored and removes unchanged artifacts entirely, measured at
  ~1.1 KB per artifact (225,542 B -> 78,003 B when dropping 135 of 192 artifacts).

Record why the CLI still does not send omit_unchanged: unchanged artifacts feed
diff.unchanged_alerts, which create_security_comment_gitlab and the FOSSA compat
issue list read unconditionally, not only under --strict-blocking. Omitting them
would silently shrink those outputs, so this needs proper gating in its own change
rather than a param tweak here.

Ref: CE-379
@lelia
lelia deployed to socket-firewall August 15, 2026 02:07 — with GitHub Actions Active
Cached diff-scan responses embed every unchanged artifact at roughly 1 KB each. On
a large dependency tree that is nearly the whole response — measured at ~11 MB for
a tree with ~10k unchanged packages — downloaded, deserialised into Package objects
and then discarded on every pull request.

omit_unchanged is honored by the API (unlike omit_license_details, which cached
responses ignore), so request it whenever no enabled output reads that half of the
comparison. Verified against the live API through the SDK: 192 artifacts -> 57.

Every consumer is behind an opt-in flag, so the gate is centralised in
Core._requires_unchanged_artifacts with the reasoning recorded there:

- --strict-blocking blocks on pre-existing issues via diff.unchanged_alerts
- --enable-gitlab-security includes them in the dependency scanning report
- --generate-license enumerates diff.packages, which must list every dependency
- --legal-format fossa reports all currently-present issues

Diff.to_dict serialises them too but has no callers. When cli_config is absent the
caller is unknown, so the full payload is kept.

Tests parametrise over every flag in that list so a new reader of
diff.unchanged_alerts or diff.packages cannot be added without also updating the
gate. The completion log reports omit_unchanged so it is visible whether the
optimisation engaged on a given run.

Ref: CE-379
@lelia
lelia deployed to socket-firewall August 15, 2026 02:19 — with GitHub Actions Active
@lelia lelia added publish-docker-preview Publish `socketdev/cli:pr-<number>` to DockerHub and removed publish-docker-preview Publish `socketdev/cli:pr-<number>` to DockerHub labels Aug 18, 2026
@lelia
lelia deployed to socket-firewall August 18, 2026 18:19 — with GitHub Actions Active
@lelia lelia added publish-docker-preview Publish `socketdev/cli:pr-<number>` to DockerHub and removed publish-docker-preview Publish `socketdev/cli:pr-<number>` to DockerHub labels Aug 18, 2026
@lelia
lelia marked this pull request as ready for review August 18, 2026 18:24
@lelia
lelia requested a review from a team as a code owner August 18, 2026 18:24
@lelia
lelia deployed to socket-firewall August 18, 2026 18:24 — with GitHub Actions Active
@lelia

lelia commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

bugbot run

Comment thread socketsecurity/core/__init__.py
@lelia
lelia deployed to socket-firewall August 18, 2026 21:28 — with GitHub Actions Active
@lelia

lelia commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit a7c6ba3. Configure here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perf improvements LGTM. ✅
I don't have context on Buildkite/GHA.

Comment thread socketsecurity/core/__init__.py Outdated
Comment thread socketsecurity/core/__init__.py Outdated
Comment thread CHANGELOG.md
Address peer review feedback by retaining pathlib.rglob trailing-slash semantics, trimming the release notes, and removing redundant implementation commentary.
@lelia
lelia deployed to socket-firewall August 19, 2026 19:36 — with GitHub Actions Active
@lelia

lelia commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit b812a18. Configure here.

@lelia
lelia merged commit 699a9a1 into main Aug 19, 2026
27 checks passed

This branch was successfully deployed

1 active deployment
socket-firewall — b812a18c Deployed Aug 19, 2026 by lelia via python-sfw-smoke-enterprise #283
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

publish-docker-preview Publish `socketdev/cli:pr-<number>` to DockerHub publish-preview Publish a CLI preview to TestPyPI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants